Added support for file URL attachments #100
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
reedmclean
changed the title
reedmclean
changed the title
reedmclean
force-pushed
the
FileUrlAttachments
branch
from
be8b82f to
9d3eb73
Compare
reedmclean
force-pushed
the
FileUrlAttachments
branch
from
9d3eb73 to
8fa4e99
Compare
Seems like a couple things have changed since we last ran the tests 2 years ago. Updated phpunit version number because some class names changed, phpunit.xml needed to be changed, reference v1.0.3 of xAPI, and fixed some Exception language that seems to have changed.
reedmclean
force-pushed
the
FileUrlAttachments
branch
from
8fa4e99 to
2d4ec21
Compare
brianjmiller
requested changes
Sep 4, 2018
| convertWarningsToExceptions="true" | ||
| processIsolation="false" | ||
| stopOnFailure="false" | ||
| syntaxCheck="false" |
Member
There was a problem hiding this comment.
What's the reason to disable the syntaxCheck?
Contributor
Author
There was a problem hiding this comment.
syntaxCheck isn't in the latest version of phpunit. I guess I removed this before I specified the version of phpunit we're using. I'll revert it.
tests/RemoteLRSTest.php
Outdated
| public function testRetrieveStatementWithFileUrlAttachments() { | ||
| $lrs = new RemoteLRS(self::$endpoint, self::$version, self::$username, self::$password); | ||
| $attachments = new Attachment(); | ||
| $pdfUrl = 'http://www.adlnet.gov/wp-content/uploads/2013/10/xAPI_v1.0.1-2013-10-01.pdf'; |
Member
There was a problem hiding this comment.
I realize this won't get fetched, but we probably don't want to point to an external resource, nor include another file in the repo for this purpose. See the tests/files/image.jpg which can be linked at https://github.com/RusticiSoftware/TinCanPHP/raw/master/tests/files/image.jpg (could also point at one of the raw github services).
brianjmiller
approved these changes
Dec 17, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right now, we will choke on any attachments that are just a file URL and not actual contents of the file being referenced. Changed the look-up for the hash to be a little more careful instead of assuming that an attachment with that hash will exist.
Getting this to run required a bit of catch up, since no one has touched this repo is a couple years. The first commit is just some small stuff like specifying a specific version of phpunit (one which is no longer supported, but that's a different PR), updating the versions of xAPI we support, updated phpunit.xml, etc.